Skip to content

Assignment 2#2

Open
elioc1341 wants to merge 3 commits intomainfrom
assignment-2
Open

Assignment 2#2
elioc1341 wants to merge 3 commits intomainfrom
assignment-2

Conversation

@elioc1341
Copy link
Owner

What changes are you trying to make? (e.g. Adding or removing code, refactoring existing code, adding reports)

  • Completed assignment 2, added code in the prompted sections.
  • Assignment 1 is also being pushed because I forgot to merge the pull request from last week. Did not change anything though.

What did you learn from the changes you have made?

  • It was my first time using numpy, so I was learning how to get comfortable with the basic functions.
  • Expanding on that, I have never used arrays in python before, so I was also playing around with the CSV's to see how the results would change with the data and getting comfortable with using them.
  • I've been a very "simple" coder forever, so this was my first time referencing functions within other functions (especially in Section 3) - it was good to know that they can be used that way to reduce the amount of work.
  • Also, since the assignment was set up to build and use the functions, it was my first time seeing code so cleanly written - I've always been used to seeing things typed out explicitly every time.

Was there another approach you were thinking about making? If so, what approach(es) were you thinking of?

  • Not really, the assignment was pretty clear that we should be leveraging previous code built in the earlier sections to achieve the final result in Section 3.
  • In section I was thinking of using a combination of readlines() and split(), but was able to achieve the result with the current code.

Were there any challenges? If so, what issue(s) did you face? How did you overcome it?

  • It was my first time using nested functions so I struggled to get them working at first. I think the main issue was that I thought I could arbitrarily throw in any variable for the function, without confirming the type of value being thrown in there. I figured it out by re-reading the assignment and also hovering my mouse over each function to figure it out.
  • I also found that I had to rewrite my code to make it more flexible - for example in section 3, I would still use the patient_summary with the first file path, and it would cause issues when I tested for other paths. (I.e. I should have changed to use file_path).

How were these changes tested?

  • I would check every file by updating the number in all_paths to check each file to determine the results while comparing by opening the files themselves. I was able to reference the 3rd, 8th, and 11th file to use as a test with a row full of 0's to test.
  • Specifically for section 2, I actually just copied the value into an Excel sheet, split into columns, and used excel formulas (AVERAGE, MAX, MIN) to compare against the returned results.

A reference to a related issue in your repository (if applicable)

  • N/A

Checklist

  • I can confirm that my changes are working as intended

@elioc1341
Copy link
Owner Author

Apologies, I only committed assignment 2 but I think not merging the results of the previous assignment caused it to also to be added in the pull request. Let me know if this is not OK.

Copy link

@anjali-deshpande-hub anjali-deshpande-hub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! I like the comments with the PR.
There is only one comment: Changes in asssignment_1.ipynb should not be with this PR. Refer to slack thread to fix this: https://uoft-dsi-certificates.slack.com/archives/C08JWD6RRU3/p1747084222526419

@elioc1341
Copy link
Owner Author

@anjali-deshpande-hub - Thanks for your help! I've reverted the changes from assignment 1.

Copy link

@anjali-deshpande-hub anjali-deshpande-hub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants